Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DesignTools] makeBlackbox() to use phys netlist not logical #1044

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

eddieh-xlnx
Copy link
Collaborator

Currently, makeBlackbox() leverages the EDIFNetlist to find all leaf cells inside a particular point of the hierarchy. In the case of encrypted or detached netlists, this is not possible. Instead, this PR proposes that only the physical netlist is used by assuming that any Cell that starts with the hierarchical name must be within the hierarchy to be blackboxed.

Am I missing a reason why this shouldn't be done? The only reason I can think of is that simply relying on the Cell name prefix may be imprecise, but is that enough of a (rare) reason to not proceed?

This PR also:

  • Fixes a situation where the intra-site routing of FF routethru-s were not being unrouted correctly
  • Depends on a new feature (in a future release) to defer the unrouting of sink pins such that they can be batched together and performed more efficiently later

@eddieh-xlnx eddieh-xlnx added the needs new release Dependent on the next future release label Aug 7, 2024
@eddieh-xlnx eddieh-xlnx requested a review from clavin-xlnx August 7, 2024 19:12
Copy link
Member

@clavin-xlnx clavin-xlnx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is possible to construct a scenario where two sibling encrypted cells could have the same prefix, but due to the wide allowance in naming conventions, could in fact be different. I'm not sure how often in practice this would occur, but IP generated from the tools seems to trigger this case.

I think if we can prove this approach works for the variety of tests and scenarios we have been looking at, I am inclined to move this direction.

@eddieh-xlnx
Copy link
Collaborator Author

It is possible to construct a scenario where two sibling encrypted cells could have the same prefix, but due to the wide allowance in naming conventions, could in fact be different.

I've been trying to think of an example of this, but struggling to. I know that the logical netlist may have hierarchical names where foo/bar/baz and foo/"bar/baz" can possibly both exist and be differentiated in the logical netlist (untested) but it's even more unclear how they can both exist in the flattened physical netlist.

If you've got an example handy please send it my way!

@clavin-xlnx
Copy link
Member

If you've got an example handy please send it my way!

Suppose you want to black box the instance foo/bar/baz and you have a cell called foo/"bar/baz"/LUT5. By looking at the physical name only, there is no way to know that the LUT5 is not part of foo/bar/baz because of the name aliasing and would get unplaced. With the logical netlist, we do have a chance of detecting this.

@eddieh-xlnx
Copy link
Collaborator Author

Suppose you want to black box the instance foo/bar/baz and you have a cell called foo/"bar/baz"/LUT5. By looking at the physical name only, there is no way to know that the LUT5 is not part of foo/bar/baz because of the name aliasing and would get unplaced. With the logical netlist, we do have a chance of detecting this.

I think I'm following. Consider two groups of leaf cells, each under a legitimate logical hierarchy:

foo/bar/baz/a
foo/bar/baz/b
foo/bar/baz/c

foo/"bar/baz"/d
foo/"bar/baz"/e
foo/"bar/baz"/f

Now these logical leaf cells lead to unique physical cell names. But I wonder what happens if the cell names collide? (i.e. {d,e,f} -> {a,b,c}). I'll have to test to see at which point Vivado starts refusing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs new release Dependent on the next future release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants